-
Notifications
You must be signed in to change notification settings - Fork 7
Improve detection of an idle indexer #6829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve detection of an idle indexer #6829
Conversation
{ | ||
// The indexer uses multiple threads for different types of work. Queue a Runnable first, and when it executes, | ||
// queue the Item | ||
SearchService.IndexTask itemTask = createTask("WaitForIndexer", new SearchService.TaskListener() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine. It's a little more complicated than necessary. I think you can use the one task=createTask(). It will fire success when all the associated items are done. So the initial addRunnable(), and the second addNoop() can be run using the same task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a single task, is there a way to guarantee that the Runnable
finishes before the Item
goes into the queue, and that you get a single notification at the end of both of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. As long as the Item
gets added before the Runnable
finishes, we'll get a single success notification when the Item
completes.
After running locally, I ended up with several of these in my server log:
|
if (res == 0 && o != this) | ||
res = (seqNum < o.seqNum ? -1 : 1); | ||
return res; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
// queue an Item on every task | ||
task.addRunnable(priority, () -> { | ||
_tasks.forEach(t -> t.addNoop(priority, latch)); | ||
_defaultTask.addNoop(priority, latch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine, but does TaskListener not work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to test the TaskListener approach and not adding to every task last week after I had made the compareTo() look at the submission order. It didn't work.
Either I made a separate fix afterwards or wasn't testing what I thought I was, as it's working today. I switched back to the TaskListener and standard JDK latch implementation. I also added some debug-level logging to make this easier next time around.
Rationale
We see intermittent failures when automated tests delete containers while indexing is still running. We need to check both
Item
andRunnable
queues. An example is Issue 53417.Changes
Runnable
and when it completes, queue theItem